Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add repr for plugins #14463

Merged
merged 5 commits into from Dec 21, 2022
Merged

Add repr for plugins #14463

merged 5 commits into from Dec 21, 2022

Conversation

LeonarddeR
Copy link
Collaborator

@LeonarddeR LeonarddeR commented Dec 20, 2022

Link to issue number:

None

Summary of the issue:

Several plugin types, such as addon objects and drivers, have a default repr that makes it impossible to identify the addon from the python console without further inspection.

Description of user facing changes

When inspecting addons, drivers and appModules from the python console, the repr will be different.

Description of development approach

Added __repr__ method to several classes

Testing strategy:

Tested inspection of the objects using the python console.

Known issues with pull request:

None known

Change log entries:

For Developers

  • Some plugin objects (e.g. drivers and add-ons) now have a more informative description in the python console.

Code Review Checklist:

  • Pull Request description:
    • description is up to date
    • change log entries
  • Testing:
    • Unit tests
    • System (end to end) tests
    • Manual testing
  • API is compatible with existing add-ons.
  • Documentation:
    • User Documentation
    • Developer / Technical Documentation
    • Context sensitive help for GUI changes
  • UX of all users considered:
    • Speech
    • Braille
    • Low Vision
    • Different web browsers
    • Localization in other languages / culture than English
  • Security precautions taken.

@LeonarddeR LeonarddeR requested a review from a team as a code owner December 20, 2022 11:40
@AppVeyorBot
Copy link

See test results for failed build of commit 2edae701da

Copy link
Member

@seanbudd seanbudd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally LGTM

@@ -563,7 +563,10 @@ def _get_productVersion(self):
return self.productVersion

def __repr__(self):
return "<%r (appName %r, process ID %s) at address %x>"%(self.appModuleName,self.appName,self.processID,id(self))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is removing the address information intended?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. I've never seen a case where having the address in repr made any sense. I can easily restore it though.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

makes sense to me

@seanbudd seanbudd merged commit e5d1844 into nvaccess:master Dec 21, 2022
@nvaccessAuto nvaccessAuto added this to the 2023.1 milestone Dec 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants